-
-
Notifications
You must be signed in to change notification settings - Fork 398
Fixed compiler output capture in JSON mode #2078
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
c293b1c
to
d2c4306
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2078 +/- ##
==========================================
+ Coverage 36.45% 36.46% +0.01%
==========================================
Files 229 229
Lines 19588 19610 +22
==========================================
+ Hits 7140 7151 +11
- Misses 11611 11618 +7
- Partials 837 841 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
@@ -184,29 +184,30 @@ const ( | |||
) | |||
|
|||
func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int) ([]byte, []byte, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got inspired by this refactoring, but do what you want with it. Since it took longer than expected it might not be that good
func getWriter(option int, isVerbose bool, candidate io.Writer) io.Writer {
getCandidateOrStdout := func() io.Writer {
if candidate != nil {
return candidate
}
return os.Stdout
}
mapOptionToWriterFunc := map[int]func() io.Writer{
Ignore: func() io.Writer {
return io.Discard
},
Show: getCandidateOrStdout,
ShowIfVerbose: func() io.Writer {
if !isVerbose {
return io.Discard
}
return getCandidateOrStdout()
},
Capture: func() io.Writer {
return &bytes.Buffer{}
},
}
return mapOptionToWriterFunc[option]()
}
func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int) ([]byte, []byte, error) {
stdoutWriter := getWriter(stdout, ctx.Verbose, ctx.Stdout)
stderrWriter := getWriter(stderr, ctx.Verbose, ctx.Stderr)
return ExecCommandWithWriters(ctx, command, stdoutWriter, stderrWriter)
}
func ExecCommandWithWriters(ctx *types.Context, command *exec.Cmd, stdout io.Writer, stderr io.Writer) ([]byte, []byte, error) {
if ctx.Verbose {
ctx.Info(PrintableCommand(command.Args))
}
command.Stdout = stdout
command.Stderr = stderr
err := command.Start()
if err != nil {
return nil, nil, errors.WithStack(err)
}
err = command.Wait()
var outbytes, errbytes []byte
if buf, ok := command.Stdout.(*bytes.Buffer); ok {
outbytes = buf.Bytes()
}
if buf, ok := command.Stderr.(*bytes.Buffer); ok {
errbytes = buf.Bytes()
}
return outbytes, errbytes, errors.WithStack(err)
}```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something to keep in mind when this subroutine will be merged with executils
and removed from legacy
.
Some instances of ctx.Stdout may now contains nil since it's no more altered during ExecCommand.
d2c4306
to
8b73eee
Compare
Please check if the PR fulfills these requirements
See how to contribute
before creating one)
our contributing guidelines
UPGRADING.md
has been updated with a migration guide (for breaking changes)What kind of change does this PR introduce?
In some cases, while running a compile with JSON output format, the stderr of the compiler is lost.
See #1698 for details
What is the current behavior?
What is the new behavior?
With the same setup:
Does this PR introduce a breaking change, and is titled accordingly?
No
Other information